fix: honor -stderrthreshold flag in klog v2#3086
fix: honor -stderrthreshold flag in klog v2#3086pierluigilenoci wants to merge 2 commits intokubesphere:mainfrom
Conversation
klog v2 defaults -logtostderr=true, which silently ignores the -stderrthreshold flag (kubernetes/klog#212). klog v2.140.0 introduced the legacy_stderr_threshold_behavior flag to fix this (kubernetes/klog#432). After every klog.InitFlags() call, set logtostderr=false and legacy_stderr_threshold_behavior=true so that -stderrthreshold works as expected. Bump k8s.io/klog/v2 from v2.130.1 to v2.140.0. Signed-off-by: Pierluigi Lenoci <pierluigilenoci@gmail.com>
kubesphere-prow
Bot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pierluigilenoci The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @pierluigilenoci! It looks like this is your first PR to kubesphere/kubekey 🎉 |
kubesphere-prow
Bot
added
the
size/S
Set legacy_stderr_threshold_behavior=false (not true) to enable the fix, and set stderrthreshold=INFO (not logtostderr=false) to preserve default behavior while allowing users to override on the command line. Signed-off-by: Pierluigi Lenoci <pierluigi.lenoci@gmail.com> Signed-off-by: Pierluigi Lenoci <pierluigilenoci@gmail.com>
|
This PR has multiple commits, and the default merge method is: squash. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the k8s.io/klog/v2 dependency to version 2.140.0 and configures the logtostderr and legacy_stderr_threshold_behavior flags in cmd/controller-manager/app/options/common.go and cmd/kk/app/options/other.go to resolve an issue where -stderrthreshold was ignored. The review feedback recommends adding error handling for the local.Set calls to prevent silent configuration failures during application startup.
| _ = local.Set("legacy_stderr_threshold_behavior", "false") | ||
| _ = local.Set("stderrthreshold", "INFO") |
There was a problem hiding this comment.
The errors returned by local.Set are being ignored. While it's unlikely to fail in this case, it's a good practice to handle errors during application initialization. A failure to set these flags would lead to incorrect logging behavior that could be difficult to debug. Consider checking the error and logging a fatal error if setting the flag fails, to ensure the application exits if it cannot be configured correctly.
if err := local.Set("logtostderr", "false"); err != nil {
klog.Fatalf("failed to set klog logtostderr flag: %v", err)
}
if err := local.Set("legacy_stderr_threshold_behavior", "true"); err != nil {
klog.Fatalf("failed to set klog legacy_stderr_threshold_behavior flag: %v", err)
}| _ = local.Set("legacy_stderr_threshold_behavior", "false") | ||
| _ = local.Set("stderrthreshold", "INFO") |
There was a problem hiding this comment.
The errors returned by local.Set are being ignored. While it's unlikely to fail in this case, it's a good practice to handle errors during application initialization. A failure to set these flags would lead to incorrect logging behavior that could be difficult to debug. Consider checking the error and logging a fatal error if setting the flag fails, to ensure the application exits if it cannot be configured correctly.
if err := local.Set("logtostderr", "false"); err != nil {
klog.Fatalf("failed to set klog logtostderr flag: %v", err)
}
if err := local.Set("legacy_stderr_threshold_behavior", "true"); err != nil {
klog.Fatalf("failed to set klog legacy_stderr_threshold_behavior flag: %v", err)
}
kubesphere-prow
Bot
added
release-note-none
and removed
do-not-merge/release-note-label-needed
labels
1 participant
What this PR does
klog v2 defaults
-logtostderr=true, which silently ignores the-stderrthresholdflag (kubernetes/klog#212).Users who set
-stderrthresholdexpect it to control which severitylevel is written to stderr, but the flag has no effect because
-logtostderrtakes precedence.klog v2.140.0 introduced the
legacy_stderr_threshold_behaviorflag to restore the intended behaviour
(kubernetes/klog#432).
Changes
cmd/kk/app/options/other.goandcmd/controller-manager/app/options/common.go: after everyklog.InitFlags()call, setlogtostderr=falseandlegacy_stderr_threshold_behavior=trueso that-stderrthresholdworks as expected.
go.mod: bumpk8s.io/klog/v2from v2.130.1 to v2.140.0.Why
Without this fix, any
-stderrthresholdvalue passed tokkorcontroller-manageris silently ignored, making it impossible tosuppress low-severity log output on stderr.
How to test
Both binaries compile and the new flag values are set programmatically
(no user-facing CLI change).